feat: 3430 - new packagings edit page based on api v3 #3475

monsieurtanuki · 2022-12-27T12:45:19Z

New file:

edit_new_packagings.dart: Edit display of a product packagings (the new api V3 version).

Impacted files:

background_task_details.dart: added specific save method call for api V3
edit_product_page.dart: added call to new page EditNewPackagings
ocr_packaging_helper.dart: minor unrelated refactoring
paged_user_product_query.dart: specific field list
product_query.dart: upgraded to api V3; added field PACKAGINGS; added specific field list for user-related searches (they do not support PACKAGINGS)
product_refresher.dart: upgraded to api V3
simple_input_page.dart: minor refactoring
simple_input_widget.dart: refactoring about making the autocomplete widget reusable
up_to_date_changes.dart: added new field packagings; minor unrelated refactoring

What

New "edit packagings page" based on api v3
Those packagings are split in material, shape, recycling status and number of units
I coded quickly without much regard for UI or localization - that would have to be done in a second step.

Screenshot

Edit product: "NEW PACKAGINGS"	new page

Fixes bug(s)

New file: * `edit_new_packagings.dart`: Edit display of a product packagings (the new api V3 version). Impacted files: * `background_task_details.dart`: added specific save method call for api V3 * `edit_product_page.dart`: added call to new page `EditNewPackagings` * `ocr_packaging_helper.dart`: minor unrelated refactoring * `paged_user_product_query.dart`: specific field list * `product_query.dart`: upgraded to api V3; added field `PACKAGINGS`; added specific field list for user-related searches (they do not support `PACKAGINGS`) * `product_refresher.dart`: upgraded to api V3 * `simple_input_page.dart`: minor refactoring * `simple_input_widget.dart`: refactoring about making the autocomplete widget reusable * `up_to_date_changes.dart`: added new field `packagings`; minor unrelated refactoring

teolemon · 2022-12-27T12:52:40Z

Great for the quick turnaround @monsieurtanuki 👏
Are Weight of one empty unit (g)and Quantity of product contained per unit causing any issues ?

codecov-commenter · 2022-12-27T12:53:22Z

Codecov Report

Merging #3475 (6010cfe) into develop (e2a8464) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3475      +/-   ##
===========================================
- Coverage    11.23%   11.09%   -0.15%     
===========================================
  Files          261      262       +1     
  Lines        12670    12830     +160     
===========================================
  Hits          1424     1424              
- Misses       11246    11406     +160

Impacted Files	Coverage Δ
...th_app/lib/background/background_task_details.dart	`0.00% <0.00%> (ø)`
...smooth_app/lib/data_models/up_to_date_changes.dart	`0.00% <0.00%> (ø)`
...pp/lib/pages/product/common/product_refresher.dart	`1.96% <0.00%> (ø)`
...oth_app/lib/pages/product/edit_new_packagings.dart	`0.00% <0.00%> (ø)`
...mooth_app/lib/pages/product/edit_product_page.dart	`0.50% <0.00%> (-0.02%)`	⬇️
...th_app/lib/pages/product/ocr_packaging_helper.dart	`0.00% <ø> (ø)`
...mooth_app/lib/pages/product/simple_input_page.dart	`0.00% <0.00%> (ø)`
...oth_app/lib/pages/product/simple_input_widget.dart	`0.00% <0.00%> (ø)`
...smooth_app/lib/query/paged_user_product_query.dart	`2.38% <0.00%> (ø)`
packages/smooth_app/lib/query/product_query.dart	`7.50% <0.00%> (-0.61%)`	⬇️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

AshAman999 · 2022-12-27T13:38:09Z

packages/smooth_app/lib/background/background_task_details.dart

+          result.status != ProductResultV3.statusWarning) {
+        throw Exception('Could not save product - ${result.errors}');
+      }
+      return;


A bit confused about the return statement here, do you think it's needed here ?
Suppose I am offline now and I did some edits to the product name and then to this packagings
So when I connect back to the internet, the first block ie the code from if condition would run, and then after successful, it would simply return and won't do the save query for the rest of the changes,

Is there something I am missing

We don't merge changes, and just save them sequentially one by one.
In the case we want to save packagings, we are going to save only this field. Therefore only in api v3, and when it's done there's nothing else to do.

We don't merge changes, and just save them sequentially one by one.

Thanks for the answer, now it's clear

monsieurtanuki · 2022-12-27T14:24:04Z

Great for the quick turnaround @monsieurtanuki 👏 Are Weight of one empty unit (g)and Quantity of product contained per unit causing any issues ?

They are not in off-dart.

teolemon · 2022-12-27T15:51:04Z

ok @monsieurtanuki, I have filed a task there

packages/smooth_app/lib/pages/product/edit_product_page.dart

teolemon

Functionality wise, it's a good first step 👍
Good to merge with a technical review from @AshAman999 or @M123-dev

Co-authored-by: Pierre Slamich <[email protected]>

M123-dev

nothing that worries me here, thanks @monsieurtanuki

M123-dev · 2022-12-27T18:51:07Z

packages/smooth_app/lib/background/background_task_details.dart

+      }
+      return;
+    }
+    final Status status = await OpenFoodAPIClient.saveProduct(
      getUser(),
      _product,


Shouldn't we remove the packaging from here, or are they ignored by the older api versions

As I said in a previous comment, save v3 can only save packagings, and the other save method can save the rest. So we need both.
Not sure that answers your question, though.

AshAman999

Found some issues with the theme and others but found out you already have todos left for em
I was trying the build on my emulator and found that the focus node, ie keyboard on the new packaging screen can't be dismissed,
Nothing blocking though, it does the job, We can fix these later on

👍 for merge from my side too

AshAman999 · 2022-12-27T19:32:12Z

packages/smooth_app/lib/background/background_task_details.dart

      getUser(),
      _product,
      language: getLanguage(),
      country: getCountry(),
    );
+    if (status.status != 1) {


Hello @M123-dev
Just out of curiosity, Can / Should we log this into sentry, Just wanted to make sure and keep a look into how many fails we do incur when saving data, would be helpful in understanding the background sync bug

I think the Exception will be automatically collected by sentry, but this is to be checked

@AshAman999 More broadly, the question is again: what should we do with failing changes? Run them forever? Ignore them? Alert the user?

I'm thinking that a alert would be more subtle for now
At least the user will know that something went wrong

About retires I am not sure that we are going to have so many of those occurrences for failures,

I guess next setry report will reveal these data

monsieurtanuki · 2022-12-27T21:06:40Z

Thank you guys for the reviews!
A good first step. It's not over.

monsieurtanuki requested a review from a team as a code owner December 27, 2022 12:45

github-actions bot assigned monsieurtanuki Dec 27, 2022

github-actions bot added the 🥫 Product page label Dec 27, 2022

monsieurtanuki requested review from M123-dev, teolemon and AshAman999 December 27, 2022 12:45

teolemon added the ✏️ Editing - Packaging input Related to the structured input of food packaging. label Dec 27, 2022

AshAman999 reviewed Dec 27, 2022

View reviewed changes

teolemon mentioned this pull request Dec 27, 2022

Add support for Weight of one empty unit (g)and Quantity of product contained per unit openfoodfacts/openfoodfacts-dart#662

Closed

teolemon reviewed Dec 27, 2022

View reviewed changes

packages/smooth_app/lib/pages/product/edit_product_page.dart Outdated Show resolved Hide resolved

teolemon approved these changes Dec 27, 2022

View reviewed changes

teolemon and others added 3 commits December 27, 2022 19:47

Merge branch 'develop' into feat/3430

f78dab3

Update packages/smooth_app/lib/pages/product/edit_product_page.dart

119a1ed

Co-authored-by: Pierre Slamich <[email protected]>

feat: 3430 - dart format

6010cfe

M123-dev approved these changes Dec 27, 2022

View reviewed changes

AshAman999 approved these changes Dec 27, 2022

View reviewed changes

AshAman999 reviewed Dec 27, 2022

View reviewed changes

monsieurtanuki merged commit 09a982a into openfoodfacts:develop Dec 27, 2022

github-actions bot mentioned this pull request Dec 27, 2022

chore(develop): release 4.2.0 #3422

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 3430 - new packagings edit page based on api v3 #3475

feat: 3430 - new packagings edit page based on api v3 #3475

monsieurtanuki commented Dec 27, 2022

teolemon commented Dec 27, 2022

codecov-commenter commented Dec 27, 2022 •

edited

Loading

AshAman999 Dec 27, 2022

monsieurtanuki Dec 27, 2022

AshAman999 Dec 27, 2022

monsieurtanuki commented Dec 27, 2022

teolemon commented Dec 27, 2022

teolemon left a comment

M123-dev left a comment

M123-dev Dec 27, 2022

monsieurtanuki Dec 27, 2022

AshAman999 left a comment

AshAman999 Dec 27, 2022

M123-dev Dec 27, 2022

monsieurtanuki Dec 27, 2022

AshAman999 Dec 28, 2022

monsieurtanuki commented Dec 27, 2022

feat: 3430 - new packagings edit page based on api v3 #3475

feat: 3430 - new packagings edit page based on api v3 #3475

Conversation

monsieurtanuki commented Dec 27, 2022

What

Screenshot

Fixes bug(s)

teolemon commented Dec 27, 2022

codecov-commenter commented Dec 27, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki commented Dec 27, 2022

teolemon commented Dec 27, 2022

teolemon left a comment

Choose a reason for hiding this comment

M123-dev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AshAman999 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki commented Dec 27, 2022

codecov-commenter commented Dec 27, 2022 •

edited

Loading